Hello @mpaolino!

This is your first pull request on the PrestaShop project. Thank you, and welcome to this Open Source community!

Actually there is more than one bug in that single conditional line.The problem why QA could not reproduce it was probably they didn't made a purchase that was higher or equals to $1000.

The rationale behind that comparison is they are trying to avoid rounding errors, but:

1. They are comparing with different number of decimal precision (number_format rounds the values).

2. They are using the inequality operator to do the comparison, which will actually convert strings to numbers, and it will round numbers and incorrectly process commas in thousands separators. This achieves the complete opposite of what was intended.
   Notably, this situation yields wrong results if the purchase exceeds 1000, or if the purchase is lower than 1000 but the paid amount has any decimals.

php > var_dump(number_format(1000, 2));
string(8) "1,000.00"

php > var_dump(number_format(1000, 0));
string(5) "1,000"

php > var_dump(number_format(100, 2));
string(6) "100.00"

php > var_dump(number_format(100, 0));
string(3) "100"

This is how PHP prior to 8.0 does the comparison on number strings: https://www.php.net/manual/en/language.operators.comparison.php

So for example:

php > var_dump(number_format(1000, 2) != number_format(1000, 0));
bool(true)

php > var_dump(number_format(100, 2) != number_format(100, 0));
bool(false)

php > var_dump(number_format(100, 2) != number_format(100.10, 0));
bool(false)

There are a number of ways of doing this right but probably PHPs strcmp will work correctly independently of all combinations, I will update this if I have the time.

@matks done. Steps to reproduce in bug report.

Hello, I just tested the change in the PaymentModule.php file and it no longer throws a payment error with the bank transfer module in Colombia currency without decimals.

https://drive.google.com/file/d/1-6ESV_2R6voEnkq3kv4fIg5MkkT6kCRF/view?usp=sharing

Thanks,

Hello,

Adding what is indicated by @mpaolino.

Please remember that there is also evidence of the bug #26671 reported with the bank transfer module where multiple users have reported the same decimal problem.

Thanks,

Hi @mpaolino , please notice that that the same constant is being used in another place of the same file:
https://github.com/PrestaShop/PrestaShop/blob/develop/classes/PaymentModule.php#L1160

Could you change that one too to avoid another PR?

Hi @mpaolino , please notice that that the same constant is being used in another place of the same file:
https://github.com/PrestaShop/PrestaShop/blob/develop/classes/PaymentModule.php#L1160

Could you change that one too to avoid another PR?

Hi.

I already pointed that out in the initial PR report. This is a bug fix for a single reported bug, what your are asking here is that I complete the refactor of PaymentModule to completely remove the constant. Although I recognize there must be another underlying bug there, this is out of scope for this PR and I don't have the time to hunt for the triggering use cases. Feel free to provide the PR as it will require a different set of test instructions.

@sowbiba I assume there is something required from me since the issue is in "Waiting for author". Can you comment on that?

Any news to be able to mix the PR?

Thanks @mpaolino

Thanks to everyone